Deprioritize old Intel Mesa Vulkan adapters by Mesa version instead of adapter name - #14583
Deprioritize old Intel Mesa Vulkan adapters by Mesa version instead of adapter name#14583warp-agent-staging[bot] wants to merge 1 commit into
Conversation
Warp freezes on the startup message on an `Intel(R) Xe Graphics (TGL GT2)` with Mesa 21.2.6: every `get_current_texture` call returns a validation error, so no frame after the first ever renders. This is the same failure as the Intel UHD (ICL GT1) report, but the name-based allowlist in `is_older_vulkan_intel_uhd_adapter` missed it because the adapter is an `Xe`/`GT2` part rather than a `UHD`/`GT1` one, even though the broken driver is identical. Replace the allowlist with a Mesa-version check: a Vulkan integrated adapter using the Mesa Intel driver on Mesa older than 21.3.6 is deprioritized, so the healthy GL adapter for the same GPU is picked instead. This only affects ranking, so the Vulkan adapter is still used when nothing else can present to the surface. Also split the stability ranking out of `adapter_stability_sort_func` into `adapter_support`, which takes `wgpu::AdapterInfo`, so adapter ranking can be unit tested without a real GPU. Fixes #14577 Co-Authored-By: Oz <oz-agent@warp.dev>
![warp-agent-staging[bot]](https://avatars.githubusercontent.com/in/2196476?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: adapter ranking by Mesa version (fixes #14577)
Verdict: approve with comments — no blocking issues found. I re-ran the implementer's validation and independently probed the new detection logic; the change does what it claims. Everything below is non-blocking polish, plus one process caveat before merge.
What I verified independently
cargo fmt -p warpui -- --check— passcargo test -p warpui --lib rendering::wgpu::resources— 7 passed, 0 failedcargo clippy -p warpui --all-targets --tests -- -D warnings— pass (clean)- Detection matches the reported adapter:
driver = "Intel open-source Mesa driver"/driver_info = "Mesa 21.2.6"satisfiesis_intel_mesa_adapter, so the reporter'sIntel(R) Xe Graphics (TGL GT2)is now rankedSupportedWithIssues. - No regression on #14482: all three previously allowlisted names (
Intel(R) HD Graphics 620,Intel(R) UHD Graphics (ICL GT1),Intel(R) UHD Graphics (TGL GT1)) still match, because the old allowlist path already required aMesa <version>driver_info, and ANV always reportsIntel open-source Mesa driveralongside it. I could not construct a realistic case that the old rule caught and the new one misses: in wgpu 30 (wgpu-hal/src/vulkan/adapter.rs)driveranddriver_infoare both populated fromVkPhysicalDeviceDriverProperties, and both fall back to"?"together, which neither the old nor the new rule matches. - Version parsing / malformed input (probed locally with throwaway tests, none panicked):
Mesa 21.3.6→ not deprioritized;Mesa 21.3.5→ deprioritized (boundary is correct and exclusive)- distro suffixes:
Mesa 21.2.6-0ubuntu0.1~20.04.2→ deprioritized;Mesa 21.3.6-0ubuntu0.1~20.04.2→ not "?","",Mesa,Mssa 21.2.6→ not deprioritized, no panicMesa wtfis&this→ deprioritized (existing conservative "unparseable ⇒ downrank" behavior, retained)
- Scoping holds: RADV (
driver = "radv") and llvmpipe (driver = "llvmpipe",DeviceType::Cpu) are untouched — llvmpipe still ranksUnsupportedvia the unchangedis_older_lavapipe_adapterbranch; discrete Intel Arc on old Mesa is excluded by theIntegratedGpucheck; WindowsIntel driver 31.0.101.xstrings are never parsed as Mesa versions. - Refactor is behavior-neutral:
adapter_supportis a pure move of the body ofadapter_stability_sort_funcwith&adapter_info→adapter_info; branch order (including the Intel rule before the llvmpipe / Mesa < 24.0.2 guard) and all early returns are unchanged, andadapter.get_info()is still called exactly once per invocation. - Fallback safety confirmed at the call site:
select_adapteronly filters onis_supported_adapter(present + format), never onAdapterSupport, and it iterates all remaining adapters callinginitialize_device. So a machine with only the old Vulkan adapter still gets it, and even if a GL adapter is ranked first but fails device/swapchain creation, the loop falls through to Vulkan. A full three-adapter ranking probe (llvmpipe/ Intel Vulkan / Intel GL) producedGl → Vulkan(Intel) → Vulkan(llvmpipe), which is the intended order.
Process caveat (not a code problem)
gh pr checks reports "all checks successful", but the substantive jobs — Formatting + Clippy, Run tests, Verify compilation with release flags, Miscellaneous checks — are all skipped because the PR is a draft. Only CodeQL/CLA/base-branch checks actually ran. My local validation covered Linux and -p warpui only; this file has cfg!(windows) branches, so please let the full matrix run (mark ready) before merging.
Non-blocking comments
See inline. Summary: an inaccurate doc comment on is_intel_mesa_adapter (it does not match Mesa GL adapters), a couple of test-coverage/wording gaps, a stale test name, and an optional robustness suggestion (vendor == 0x8086 instead of substring matching).
Scope
Scope is appropriately narrow — Vulkan + integrated + Mesa Intel + version-gated, i.e. narrower than the issue's suggestion of "any Vulkan adapter on old Mesa". Worth being explicit that the blast radius is still materially wider than the 3-name allowlist: every Intel iGPU on Mesa < 21.3.6 (all of Ubuntu 20.04) now prefers GL, and because the stability sort is the dominant sort key it also overrides an explicit in-app Vulkan backend preference on those machines (pre-existing behavior, newly reachable by more users). The WGPU_BACKEND=vulkan env escape hatch still works since it filters enumeration. I think that trade is right given the failure mode is a permanently frozen window, but it deserves a human sign-off.
The deferred runtime-recovery suggestion (recreate the renderer on GL after N consecutive GetSurfaceTextureError::Validation failures) is clearly documented in the PR description as out of scope — that's the right call for this PR, but it currently exists nowhere except the PR body. Please file a follow-up issue (or leave a TODO next to the GetSurfaceTextureError::Validation arm in Resources::get_surface_texture) so it isn't silently dropped once this merges; it's the only thing that would cover the non-Intel cases (#11003) and any future adapter this heuristic misses.
| /// - `Intel(R) UHD Graphics (TGL GT1)` — window flashing/flicker on older Mesa (PLAT-599, GH #4533) | ||
| /// Returns whether the provided adapter uses the Mesa open-source Intel driver (ANV for Vulkan, | ||
| /// Iris/i965 for GL). | ||
| fn is_intel_mesa_adapter(adapter_info: &wgpu::AdapterInfo) -> bool { |
There was a problem hiding this comment.
Doc comment is inaccurate for GL: this helper never matches a Mesa GL adapter.
In wgpu 30 the GL backend builds AdapterInfo in wgpu-hal/src/gles/adapter.rs::make_info, which sets only name, vendor and driver_info — driver is left as the default empty string (that's why the reporter's log shows Driver: Unknown for the GL adapter). Since is_intel_mesa_adapter requires driver.contains("Mesa"), it returns false for Mesa Intel(R) Xe Graphics (TGL GT2) on GL.
That's harmless today because the only caller already gates on Backend::Vulkan, but the comment invites a future caller to reuse this for GL and silently get false. Suggest either narrowing the comment to the Vulkan/ANV case or renaming to something like is_intel_mesa_vulkan_adapter.
| // "Mesa" in the driver name keeps us from misinterpreting non-Mesa (e.g. Windows) Intel driver | ||
| // version strings as Mesa versions. | ||
| adapter_info.driver.contains("Mesa") | ||
| && (adapter_info.driver.contains("Intel") || adapter_info.name.contains("Intel")) |
There was a problem hiding this comment.
Optional robustness suggestion: AdapterInfo::vendor is populated with the PCI vendor ID by both the Vulkan and GL backends (db::intel::VENDOR = 0x8086), so the Intel half of this test could be adapter_info.vendor == 0x8086 instead of substring matching on driver/name. That's immune to unusual/absent driverName strings and to any adapter that merely happens to have "Intel" in its name (e.g. a zink-style adapter reporting Mesa ... as its driver and an Intel renderer string currently matches here).
Not a blocker — the current strings do match the reported hardware, and switching would require setting vendor in the test helper (it's hardcoded to 0).
| } | ||
|
|
||
| #[test] | ||
| fn test_is_unsupported_intel_uhd_adapter() { |
There was a problem hiding this comment.
Nit: this test no longer exercises anything UHD-specific — consider renaming to match the new rule (e.g. test_is_unsupported_intel_mesa_adapter), since the name is now the only remaining reference to the removed allowlist.
| /// The Intel graphics family/GT tier in the adapter name must not affect the result: matching is | ||
| /// based on the Mesa driver version. See https://github.com/warpdotdev/warp/issues/14577. | ||
| #[test] | ||
| fn test_older_vulkan_intel_mesa_adapters_are_matched_regardless_of_family() { |
There was a problem hiding this comment.
Coverage gap: nothing here pins the version boundary. The interesting cases for a rule that is now purely version-keyed are Mesa 21.3.5 (deprioritized) and Mesa 21.3.6 (not deprioritized, i.e. the comparison is strictly-less-than) — the existing cases only use 21.2.6 / 23.2.6 / 24.0.2, which would still pass if the threshold silently moved by a patch release.
I verified locally that the current code gets both right, plus distro-suffixed strings (Mesa 21.2.6-0ubuntu0.1~20.04.2 → true, Mesa 21.3.6-0ubuntu0.1~20.04.2 → false, which is the exact form Ubuntu 20.04 can report). Worth adding those four as explicit cases.
| AdapterSupport::Supported | ||
| ); | ||
| // With both adapters equally supported, the stability sort is a no-op and the earlier | ||
| // backend-priority sort (which prefers Vulkan on Linux) decides. |
There was a problem hiding this comment.
This comment overstates what the test proves. rank_by_support only applies the stability sort, so the backend-priority sort is never executed here — with both adapters equal, sorted_by_key is stable and ranked[0] is simply whichever element was pushed into the vec first. The assertion would pass even if backend priority preferred GL (as it effectively can on Windows, where DX12/Vulkan/GL ordering differs).
Suggest either rewording to "the stability sort is a no-op, so input order is preserved" or asserting on adapter_support(&gl, ...) == adapter_support(&vulkan, ...) instead of on ordering.
1 participant
Description
Warp freezes on the
Starting <shell>...message on anIntel(R) Xe Graphics (TGL GT2)running Mesa 21.2.6: the Vulkan adapter is selected, then everyget_current_texture()call returns a validation error, so no frame after the first one is ever presented. A healthy GL adapter for the same GPU is enumerated right behind it and passed over.This is the same failure mode as the Intel UHD (ICL GT1) report fixed by #14482, but the name-based allowlist in
is_older_vulkan_intel_uhd_adaptermissed this adapter on both axes — family (Xe, notUHD) and GT tier (GT2, notGT1) — even though the broken driver is identical (Mesa 21.2.6, belowMIN_SUPPORTED_INTEL_UHD_VERSION= 21.3.6).Changes in
crates/warpui/src/rendering/wgpu/resources.rs:is_older_vulkan_intel_mesa_adapterdeprioritizes any Vulkan integrated adapter using the Mesa Intel driver whose Mesa version is belowMIN_SUPPORTED_INTEL_MESA_VERSION(renamed fromMIN_SUPPORTED_INTEL_UHD_VERSION, value unchanged at 21.3.6). This subsumes the previous allowlist entries and covers future Intel families/GT tiers on the same broken drivers.MesaandIntel, or a Mesa driver with an Intel adapter name) rather than "any Vulkan adapter on old Mesa": that keeps non-Intel Mesa stacks (e.g. RADV) on their current behavior, and avoids parsing non-Mesa Intel driver version strings (e.g. WindowsIntel driver 31.0.101.x) as Mesa versions, which would spamreport_error!.adapter_stability_sort_funcintoadapter_support(&wgpu::AdapterInfo, ...)so adapter ranking can be unit tested without a real GPU. No behavior change.The change only affects ranking, not filtering: the deprioritized adapter is still
SupportedWithIssues(notUnsupported), so if no GL adapter can present to the surface, Warp still falls back to the old Vulkan adapter rather than failing to open a window.Not implemented here (deliberately out of scope): the reporter's second suggestion of recreating the renderer on the GL backend after N consecutive
GetSurfaceTextureError::Validationfailures. That is a larger change to renderer lifetime management; the ranking fix addresses the reported configuration, and runtime recovery can be tackled separately if we see machines where the wrong adapter is still chosen.Linked Issue
Fixes #14577
factory-auto-implement.Testing
Added unit tests in
crates/warpui/src/rendering/wgpu/resources_tests.rscovering the reported adapter strings:Intel(R) Xe Graphics (TGL GT2)+Mesa 21.2.6on Vulkan is deprioritized, and the GL adapter (Mesa Intel(R) Xe Graphics (TGL GT2)) ranks ahead of it.SupportedWithIssues(strictly better thanUnsupported) and is still selected.Xe/UHD/HD/ADLnames all match on old Mesa).test_is_unsupported_intel_uhd_adapter/test_is_unsupported_llvmpipe_adaptercoverage retained.Commands run (all pass):
cargo fmt -p warpui -- --checkcargo check -p warpui --lib --testscargo test -p warpui --lib rendering::wgpu::resources— 7 passed, 0 failedcargo clippy -p warpui --all-targets --tests -- -D warningsI have manually tested my changes locally with
./script/run— not possible: this reproduces only on Intel Tiger Lake GT2 hardware with Mesa 21.2.6, which is not available in this environment. Adapter ranking is covered by the unit tests above.Agent Mode
Conversation: https://staging.warp.dev/conversation/2a250cc3-5457-4a9a-90a1-65d0e196ed53
Run: https://oz.staging.warp.dev/runs/019fb80d-abad-702f-9561-f81a5c0b2192
This PR was generated with Oz.